Enable colours on GitHub Actions by default #7462

hugovk · 2020-07-08T08:56:27Z

Workaround to fix #7443.

GitHub Actions doesn't provide a tty, so many tools that normally output in colour instead output in monochrome: actions/runner#241.

GitHub say it's low in their backlog and they have no plans to fix it (actions/runner#241 (comment)), so instead of requiring every pytest user to explicitly force colours for pytest, improve the pytest colour detection using the GITHUB_ACTIONS env var:

GITHUB_ACTIONS Always set to true when GitHub Actions is running the workflow. You can use this variable to differentiate when tests are being run locally or by GitHub Actions.

https://docs.github.com/en/actions/configuring-and-managing-workflows/using-environment-variables#default-environment-variables

Test using this branch:

https://github.com/hugovk/test/runs/848991439?check_suite_focus=true#step:5:11

The-Compiler

Seems reasonable to me, especially since we handle some special cases there already anyways (e.g. disabling markup with Jython on Windows).

bluetech

LGTM.

BTW the java condition is no longer needed as we don't support Jython (Python2 only AFAIK), so can remove it as well if you feel like it.

bluetech · 2020-07-08T11:24:27Z

src/_pytest/_io/terminalwriter.py

@@ -32,7 +32,7 @@ def should_do_markup(file: TextIO) -> bool:
        and file.isatty()
        and os.environ.get("TERM") != "dumb"
        and not (sys.platform.startswith("java") and os._name == "nt")
-    )
+    ) or os.environ.get("GITHUB_ACTIONS") == "true"


I think it will be a little clearer as a separate if before the return (can leave as is if you prefer)

Sounds like a good idea, since we then handle all the special cases before the "ordinary" one.

The-Compiler · 2020-07-08T11:53:53Z

src/_pytest/_io/terminalwriter.py

@@ -32,7 +32,7 @@ def should_do_markup(file: TextIO) -> bool:
        and file.isatty()
        and os.environ.get("TERM") != "dumb"
        and not (sys.platform.startswith("java") and os._name == "nt")
-    )
+    ) or os.environ.get("GITHUB_ACTIONS") == "true"


Sounds like a good idea, since we then handle all the special cases before the "ordinary" one.

The-Compiler · 2020-07-08T11:54:30Z

src/_pytest/_io/terminalwriter.py

@@ -32,7 +32,7 @@ def should_do_markup(file: TextIO) -> bool:
        and file.isatty()
        and os.environ.get("TERM") != "dumb"
        and not (sys.platform.startswith("java") and os._name == "nt")


Seems fine to me to remove this if we don't support Jython anymore - looks like it was originally added in pytest-dev/py@536252c some 10 years ago 😆

nicoddemus

Cool, thanks!

asottile

Hmmm I'm actually -1 on this. the environment variable doesn't tell whether or not you're piped or other possible things where color is inappropriate and it's extremely special cased for github actions

nicoddemus · 2020-07-08T21:09:55Z

the environment variable doesn't tell whether or not you're piped or other possible things where color is inappropriate

But if you are piped you don't get a atty, right? We should then adapt the check to take that in account instead of forcing it everytime.

Your point about being specific is correct, but if we can get this correct in all cases I still see it as a win as every user will get this for free without additional configuration.

asottile · 2020-07-08T21:48:10Z

the environment variable doesn't tell whether or not you're piped or other possible things where color is inappropriate

But if you are piped you don't get a atty, right? We should then adapt the check to take that in account instead of forcing it everytime.

Your point about being specific is correct, but if we can get this correct in all cases I still see it as a win as every user will get this for free without additional configuration.

right, but if you're running in github actions, you can't discern whether that output is going to the log or is being piped into something else since both cases would be (GITHUB_ACTIONS present, no tty)

The-Compiler · 2020-07-09T09:01:14Z

This is a valid concern I'm afraid... and I can't think of any way to find out whether we're writing to a pipe/file or to the standard output on GitHub Actions. On the other hand, I wonder how common the case of someone piping pytest output somewhere on GitHub Actions would be.

Alternatively, maybe this should go into some pytest-github-actions plugin I guess? I've been toying with the idea of writing such a plugin to do output grouping similar to pytest-travis-fold, but as usual, haven't found the time yet 😆

nicoddemus

right, but if you're running in github actions, you can't discern whether that output is going to the log or is being piped into something else since both cases would be (GITHUB_ACTIONS present, no tty)

Ahh got it, while running in GitHub actions we never have an tty, because we are being piped, of course. Thanks for the clarification.

In that case I agree we should probably turn down this PR.

Perhaps we should add a blurb somewhere in the docs mentioning --color=yes when running in CI services such as GHA, Travis, AppVeyor, etc? I don't think we have anything in that sense currently.

The-Compiler · 2020-07-09T11:18:58Z

I think this is only a problem with GHA, as others somehow provide a fake TTY. FWIW I ended up using this at the top of the workflow file:

env:
  PY_COLORS: "1"

that way, colors will be enabled for both pytest and tox, and it won't influence other places where commandline arguments are passed or handled.

nicoddemus · 2020-07-10T11:50:21Z

Let's close this for now, thanks @hugovk for giving it a go anyway! 👍

hugovk · 2020-07-10T11:54:44Z

Sure, no problem!

umarcor · 2020-07-10T12:06:40Z

Alternatively, maybe this should go into some pytest-github-actions plugin I guess? I've been toying with the idea of writing such a plugin to do output grouping similar to pytest-travis-fold, but as usual, haven't found the time yet 😆

@The-Compiler, we are already doing that in https://github.com/ghdl/ghdl-cosim/blob/master/test.py#L25-L30. See output: https://github.com/ghdl/ghdl-cosim/runs/854521453?check_suite_focus=true#step:4:13. Unfortunately, there is an annoyance which I could not solve: when pytest is used to execute makefiles, everything is correct; but when executing other python scripts, the output/log is empty. Compare https://github.com/ghdl/ghdl-cosim/runs/854521453?check_suite_focus=true#step:4:174 and any other of the "Log" collapsible groups. That's the reason that has prevented me from "announcing" the solution; which is really simple and effective otherwise.

The-Compiler · 2020-07-10T13:03:58Z

@umarcor Interesting! I have no idea why that is. FWIW what I'd be interested in is a solution collapsing the output of pytest's own test failure output.

umarcor · 2020-07-10T15:27:52Z

FWIW what I'd be interested in is a solution collapsing the output of pytest's own test failure output.

All you need to do is print ##[group] before and ##[endgroup]. However, be aware that nested groups are NOT supported.

The-Compiler · 2020-07-10T15:41:04Z

All you need to do is print ##[group] before and ##[endgroup]. However, be aware that nested groups are NOT supported.

Note you're using an old/alternative/depecated? syntax there, that should probably be ::group::title and ::endgroup:: instead.

umarcor · 2020-07-10T17:02:10Z

Yes, you are correct. It's an old, probably deprecated, syntax. Sorry about that. I'll fix it on my side.

hugovk added 2 commits July 8, 2020 11:29

Enable colours on GitHub Actions

9667a32

Add changelog

cc9cbff

hugovk mentioned this pull request Jul 8, 2020

No colors by default on GitHub Actions #7443

Closed

The-Compiler approved these changes Jul 8, 2020

View reviewed changes

bluetech approved these changes Jul 8, 2020

View reviewed changes

The-Compiler reviewed Jul 8, 2020

View reviewed changes

nicoddemus approved these changes Jul 8, 2020

View reviewed changes

asottile requested changes Jul 8, 2020

View reviewed changes

hugovk added 2 commits July 8, 2020 22:15

Remove redundant check; Jython is no longer supported

36eaf42

Refactor: handle special cases before 'ordinary' one

62b6581

hugovk mentioned this pull request Jul 8, 2020

Add support for NO_COLOR and FORCE_COLOR #7466

Merged

Refactor to remove duplication

6f9d6a3

nicoddemus requested changes Jul 9, 2020

View reviewed changes

nicoddemus closed this Jul 10, 2020

hugovk deleted the gha-enable-colours branch July 10, 2020 11:54

kousu mentioned this pull request Feb 2, 2021

Port CI to Github Actions spinalcordtoolbox/spinalcordtoolbox#3125

Merged

7 tasks

alexrashed mentioned this pull request May 3, 2024

decrease test logging verbosity localstack/localstack#10761

Merged

2 tasks

jusiskin mentioned this pull request Oct 16, 2024

chore: use pytest automatic color output detection aws-deadline/deadline-cloud-worker-agent#440

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable colours on GitHub Actions by default #7462

Enable colours on GitHub Actions by default #7462

hugovk commented Jul 8, 2020

The-Compiler left a comment

bluetech left a comment

bluetech Jul 8, 2020

The-Compiler Jul 8, 2020

hugovk Jul 8, 2020

The-Compiler Jul 8, 2020

The-Compiler Jul 8, 2020

hugovk Jul 8, 2020

nicoddemus left a comment

asottile left a comment

nicoddemus commented Jul 8, 2020

asottile commented Jul 8, 2020

The-Compiler commented Jul 9, 2020

nicoddemus left a comment

The-Compiler commented Jul 9, 2020

nicoddemus commented Jul 10, 2020

hugovk commented Jul 10, 2020

umarcor commented Jul 10, 2020

The-Compiler commented Jul 10, 2020

umarcor commented Jul 10, 2020

The-Compiler commented Jul 10, 2020

umarcor commented Jul 10, 2020

Enable colours on GitHub Actions by default #7462

Enable colours on GitHub Actions by default #7462

Conversation

hugovk commented Jul 8, 2020

The-Compiler left a comment

Choose a reason for hiding this comment

bluetech left a comment

Choose a reason for hiding this comment

bluetech Jul 8, 2020

Choose a reason for hiding this comment

The-Compiler Jul 8, 2020

Choose a reason for hiding this comment

hugovk Jul 8, 2020

Choose a reason for hiding this comment

The-Compiler Jul 8, 2020

Choose a reason for hiding this comment

The-Compiler Jul 8, 2020

Choose a reason for hiding this comment

hugovk Jul 8, 2020

Choose a reason for hiding this comment

nicoddemus left a comment

Choose a reason for hiding this comment

asottile left a comment

Choose a reason for hiding this comment

nicoddemus commented Jul 8, 2020

asottile commented Jul 8, 2020

The-Compiler commented Jul 9, 2020

nicoddemus left a comment

Choose a reason for hiding this comment

The-Compiler commented Jul 9, 2020

nicoddemus commented Jul 10, 2020

hugovk commented Jul 10, 2020

umarcor commented Jul 10, 2020

The-Compiler commented Jul 10, 2020

umarcor commented Jul 10, 2020

The-Compiler commented Jul 10, 2020

umarcor commented Jul 10, 2020